Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SelectionList #2652

Merged
merged 105 commits into from
May 25, 2023
Merged

Add SelectionList #2652

merged 105 commits into from
May 25, 2023

Conversation

davep
Copy link
Contributor

@davep davep commented May 25, 2023

The SelectionList is a child of OptionList, combining the look and feel of CheckBox, but done as a single widget. It is, in effect, to CheckBox what RadioSet is to RadioButton, but done in a way that is as lightweight as possible. Here it is in action:

Screen.Recording.2023-05-25.at.10.09.42.mov

This PR should satisfy the requirements of #2518 and #2379.

davep added 30 commits May 11, 2023 16:19
Nothing here actually implements a selection list yet; this just sets out
the very basic framework of the widget, as it inherits form OptionList. The
key things here are:

1. It introduces a Selection class, which is an Option with a typed value.
2. The SelectionList is also typed and expects Selections of that type.
I think I'm going to give up on basing this off OptionList. It's close
enough that inheriting from it and doing more makes some sense, but it's
also just far enough away that it's starting to feel like it's more work
that is worthwhile and it'll be easier to hand-roll something fresh.
More experimenting with overriding OptionList, and rather than trying to
swap out and around the prompt under the hood, I got to thinking that it
made more sense to perhaps override render_line.

So far so good...
I don't need this any more.
This isn't needed any more now that I'm doing everything in render_line.
Now that I'm no longer having to dodge issues with getting component classes
before the DOM has spun up, I can go back to the simpler method of setting
up the selections.

This also means I can drop Mount handling.
At some point I should go through these styles and start to collapse/dedupe
them.
Mostly I feel it makes sense to have the value first, and the actual prompt
second (based on no reason at all); but given that Select does it prompt
then value, this should conform to the same approach.
It would be nice to just inherit form the OptionList messages, but the
naming of the properties wouldn't quite make sense, and there's also the
generic typing issue too. So here I start to spin up my own messages down
here.

Also, as an initial use of this, grab the OptionList highlight message and
turn it onto one of out own.
In doing so, change up how the toggling happens.
The code worked and was fine, but pyright was getting upset at the typing.
This clears that up.
The developer using this may wish to react to UI changes being made, but
they may also want to just know when the collection of selected values has
changed -- this could happen via code so won't get any UI/IO messages. So
this adds a message that is always sent when a change to the collection of
selected values happens.
The developer could remove an option that is selected, so we need to catch
that this has happened and update the collection of selected values.
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some typos, a couple of code-related questions/suggestions, but overall looking great!

docs/widgets/selection_list.md Outdated Show resolved Hide resolved
docs/widgets/selection_list.md Outdated Show resolved Hide resolved
docs/widgets/selection_list.md Outdated Show resolved Hide resolved
docs/widgets/selection_list.md Outdated Show resolved Hide resolved
src/textual/widgets/_selection_list.py Outdated Show resolved Hide resolved
src/textual/widgets/_selection_list.py Show resolved Hide resolved
tests/selection_list/test_selection_messages.py Outdated Show resolved Hide resolved
tests/selection_list/test_selection_messages.py Outdated Show resolved Hide resolved
tests/snapshot_tests/test_snapshots.py Outdated Show resolved Hide resolved
davep and others added 8 commits May 25, 2023 13:04
It's sort moved on from been about check boxen.

Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Co-authored-by: Rodrigo Girão Serrão <[email protected]>
Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Feel free to merge once you've been Rodrigoed.

docs/widgets/selection_list.md Outdated Show resolved Hide resolved
docs/widgets/selection_list.md Outdated Show resolved Hide resolved
@davep davep requested a review from rodrigogiraoserrao May 25, 2023 14:02
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've been Rodrigoed!

Looks good to me :)

@davep davep merged commit ea8c603 into Textualize:main May 25, 2023
@davep davep deleted the multiselect branch May 25, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement MultiSelect Create a RadioSet-like interface for checkbox groups
3 participants